You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
getNotificationTimeoutMs(source) used strict lowercase comparison against "calendar", falling back to 30s detection timeout when source was capitalized or padded with whitespace (e.g. "Calendar").
NotificationDismissTimer.prototype.start(durationMs) did not sanitize durationMs, allowing non-finite or non-positive numbers to disrupt timer scheduling.
NotificationDismissTimer threw a TypeError when the timer expired if this._onTimeout was missing or not a function.
Solution
Normalized source with trim().toLowerCase().
Sanitized durationMs to ensure a positive finite number before passing to _arm.
Guarded callback invocation with typeof this._onTimeout === "function".
Added unit tests in test/helpers/notificationTimer.test.js.
Thanks @hsusul — closing along with #1807 (see the #1840 note for the general convention).
source is an internal two-value enum: the only producers are literal "audio" and "calendar" strings inside meetingDetectionEngine.js, and it never crosses IPC or persistence on the way to this helper. Worth noting the engine itself strict-compares source === "calendar" in two other places (pref gating and the Join button) — so normalizing in just this one reader would imply a tolerance the rest of the pipeline doesn't have. Similarly, the timer's only production start() call receives one of two constants, and the constructor always gets an inline callback — the fallbacks would convert a loud future bug into a notification that silently never dismisses.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1807
Problem
In
src/helpers/notificationTimer.js:getNotificationTimeoutMs(source)used strict lowercase comparison against"calendar", falling back to 30s detection timeout whensourcewas capitalized or padded with whitespace (e.g."Calendar").NotificationDismissTimer.prototype.start(durationMs)did not sanitizedurationMs, allowing non-finite or non-positive numbers to disrupt timer scheduling.NotificationDismissTimerthrew aTypeErrorwhen the timer expired ifthis._onTimeoutwas missing or not a function.Solution
sourcewithtrim().toLowerCase().durationMsto ensure a positive finite number before passing to_arm.typeof this._onTimeout === "function".test/helpers/notificationTimer.test.js.Verification
node --test test/helpers/notificationTimer.test.js(passes, 10/10 tests)npm run typecheck(passes, 0 errors)npm run lint(passes, 0 errors)npm run i18n:check(passes)npm run build:renderer(passes)git diff --check(clean)